Found on Ubuntu 16.04 with m-c 20170608162720 e61060be364 This test case requires fuzzPriv plugin. It should also work with enabling screen reader enabled. Assertion failure: origContainer == prevChild->Parent() (Broken tree), at src/accessible/generic/DocAccessible.cpp:2176 #0 0x7fbd1a960b16 in mozilla::a11y::DocAccessible::PutChildrenBack(nsTArray<RefPtr<mozilla::a11y::Accessible> >*, unsigned int) src/accessible/generic/DocAccessible.cpp:2176:11 #1 0x7fbd1a95fb67 in mozilla::a11y::DocAccessible::DoARIAOwnsRelocation(mozilla::a11y::Accessible*) src/accessible/generic/DocAccessible.cpp:2136:3 #2 0x7fbd1a907df2 in mozilla::a11y::NotificationController::WillRefresh(mozilla::TimeStamp) src/accessible/base/NotificationController.cpp:809:18 #3 0x7fbd187a1870 in nsRefreshDriver::Tick(long, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:1831:12 #4 0x7fbd187aac2e in mozilla::RefreshDriverTimer::TickRefreshDrivers(long, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) src/layout/base/nsRefreshDriver.cpp:301:7 #5 0x7fbd187aa9fd in mozilla::RefreshDriverTimer::Tick(long, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:322:5 #6 0x7fbd187ae875 in mozilla::VsyncRefreshDriverTimer::RunRefreshDrivers(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:754:5 #7 0x7fbd187ad366 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:667:35 #8 0x7fbd187a9007 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::ParentProcessVsyncNotifier::Run() src/layout/base/nsRefreshDriver.cpp:513:20 #9 0x7fbd1314c35f in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1406:14 #10 0x7fbd13156d20 in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:472:10 #11 0x7fbd13ca4075 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:96:21 #12 0x7fbd13bf08e7 in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:238:10 #13 0x7fbd13bf0779 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:211:3 #14 0x7fbd182c2c2a in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:156:27 #15 0x7fbd1aec3331 in nsAppStartup::Run() src/toolkit/components/startup/nsAppStartup.cpp:283:30 #16 0x7fbd1b01e6c2 in XREMain::XRE_mainRun() src/toolkit/xre/nsAppRunner.cpp:4569:22 #17 0x7fbd1b0202fe in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:4749:8 #18 0x7fbd1b0211e8 in XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:4844:21 #19 0x4eca88 in do_main(int, char**, char**) src/browser/app/nsBrowserApp.cpp:236:22 #20 0x4ec3a0 in main src/browser/app/nsBrowserApp.cpp:309:16 #21 0x7fbd2fd7682f in __libc_start_main /build/glibc-9tT8Do/glibc-2.23/csu/../csu/libc-start.c:291 #22 0x41e0d4 in _start (m-c-1496939240-asan-debug/firefox+0x41e0d4)

(In reply to Eitan Isaacson [:eeejay] from comment #1) > Created attachment 8891094 [details] [diff] [review] > Allow <select> accessible children to be put back in list accessible. > r?surkov I'm not sure how it works at all if MoveChild operates on a wrong parent. Should we replace GetAccessibleContainer or TrueContainer method similary to AccessibleOrTrueContainer method?

We do this one line later: origContainer = prevChild->Parent(); This puts it in the correct container, the assert just needed to be updated to accept that. It was a strange assert beforehand: "x should equal y. ok, now assign x to y".

(In reply to Eitan Isaacson [:eeejay] from comment #3) > We do this one line later: > origContainer = prevChild->Parent(); right, missed that part anyway, would it be more correct to go with TrueContainer from the start? > It was a strange assert beforehand: "x should equal y. ok, now assign x to > y". the point is tt happens but it shouldn't happen, this is the assert-and-then-fix approach.

(In reply to alexander :surkov from comment #4) > (In reply to Eitan Isaacson [:eeejay] from comment #3) > > We do this one line later: > > origContainer = prevChild->Parent(); > > right, missed that part > > anyway, would it be more correct to go with TrueContainer from the start? Perfect. Didn't see that as an option.

Comment on attachment 8891543 [details] [diff] [review] Allow <select> accessible children to be put back in list accessible. r?surkov Review of attachment 8891543 [details] [diff] [review]: ----------------------------------------------------------------- ::: accessible/generic/DocAccessible.cpp @@ +2204,5 @@ > // Unset relocated flag to find an insertion point for the child. > child->SetRelocated(false); > > int32_t idxInParent = -1; > + Accessible* origContainer = AccessibleOrTrueContainer(content->GetParentNode()); I would probably add new TrueContainer method, but non document child has to have a content if it's in the document for sure. This code has a dependency on it already: walker.Seek assumes a non-null node.

Opted out of adding another DocAccessible method. Removed the "containers" array that is not used in PutChildrenBack,

Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/2142989aace8 Allow <select> accessible children to be put back in list accessible. r=surkov

Pushed by eisaacson@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/0d3bdb80960d Fix eslint issues in test. r=me

Backed this and the followup out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=119803943&repo=mozilla-inbound https://hg.mozilla.org/integration/mozilla-inbound/rev/412c52914a1b4035c722d4c36002c2f6dca39e70

Hrm, looks like the followup actually fixed the test failure. Feel free to reland this with the followup folded in.

Test fixed and eslinted. Attaching here for relanding because the tree is currently closed.

Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/7a7d0f161888 Allow <select> accessible children to be put back in list accessible. r=surkov

AFAICT, this goes back a ways. We're too late to backport to 55, but is there enough user impact to justify consideration for ESR52?

No, I don't think so. Although the assertion fails, we recover and do the right thing in non-debug builds.